Skip to content

manage focus to/from/within password reset modal appropriately - #1065

Merged
adampalay merged 3 commits into
masterfrom
adam/password-modal
Sep 23, 2013
Merged

manage focus to/from/within password reset modal appropriately#1065
adampalay merged 3 commits into
masterfrom
adam/password-modal

Conversation

@adampalay

Copy link
Copy Markdown
Contributor

Shamelessly appropriates @antoviaque's work on https://github.com/edx/edx-platform/pull/902/ and applies it to the "forgot password" modal.

Now, when you click "forgot password", focus is shifted to the new modal's "x" button. Tabbing now appropriately cycles through the modal, and when the modal is exited, focus is returned to the "forgot modal" link.

@talbs @singingwolfboy

(addresses https://edx-wiki.atlassian.net/browse/LMS-1166)

@talbs

talbs commented Sep 19, 2013

Copy link
Copy Markdown
Contributor

Looks good to me. Thx for the help! 👍

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you either set a constant named TAB_KEY and use that, or add a comment explaining that 9 corresponds to the tab key? JS key codes are always super confusing.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use ID instead of class for forgot-password-modal, to match the rest of the file. In fact, I'd suggest that you remove the confusing forgot-password-modal class from the element, and see if everything still works; based on a quick search through the source code, I don't see that class being referred to anywhere.

@adampalay

Copy link
Copy Markdown
Contributor Author

@talbs , I looked through the code, and the class "forgot-password-modal" isn't used anywhere. Nothing seems to break by removing it. Are you ok with getting rid of it?

@talbs

talbs commented Sep 20, 2013

Copy link
Copy Markdown
Contributor

@adampalay, if things are working fine without, I'm fine with losing it.

@singingwolfboy

Copy link
Copy Markdown
Contributor

👍 Merge it.

adampalay added a commit that referenced this pull request Sep 23, 2013
manage focus to/from/within password reset modal appropriately
@adampalay
adampalay merged commit 89c94b4 into master Sep 23, 2013
@adampalay
adampalay deleted the adam/password-modal branch September 23, 2013 13:36
chrisrossi pushed a commit to jazkarta/edx-platform that referenced this pull request Mar 31, 2014
…ion-announcement

Update caption of photo on Gates Foundation announcement
tomaszgy pushed a commit to open-craft/openedx-platform that referenced this pull request May 7, 2018
iloveagent57 pushed a commit that referenced this pull request Feb 26, 2024
Co-authored-by: Usama Sadiq <usama.sadiq@arbisoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants